app/upgrades.go (1)

22-25: Add a release-time guard for the mainnet bridge address.

mainnetCroBridgeContractAddress still defaults to "", and the current upgrade tests explicitly accept that value. If this is missed during a mainnet cut, v1.8 will silently leave SendCroToIbc disabled chain-wide after upgrade. Consider making CI or release packaging fail when this constant is empty for the mainnet build.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/upgrades.go` around lines 22 - 25, The constant
mainnetCroBridgeContractAddress is left as an empty string which silently
disables SendCroToIbc; add a release-time guard that fails fast when building or
running a mainnet release: implement a validation check (e.g. in an init() or an
upgrade validation function) that asserts mainnetCroBridgeContractAddress is
non-empty for mainnet builds and triggers a clear panic/exit or returns an error
during startup/packaging, or add a CI/release packaging step to reject builds
where mainnetCroBridgeContractAddress == ""; reference the symbol
mainnetCroBridgeContractAddress and the SendCroToIbc hook to locate where to
enforce the guard.

x/cronos/keeper/evmhandlers_test.go (1)

368-373: ⚡ Quick win

Use SendCroToIbcEvent ABI in TestSendCroToIbcHandler payload setup.

These cases currently pack data with SendToIbcEvent. It works only while both event input layouts stay identical, which weakens this test’s contract-specific coverage.

Proposed patch

-                input, _ := evmhandlers.SendToIbcEvent.Inputs.NonIndexed().Pack(
+                input, _ := evmhandlers.SendCroToIbcEvent.Inputs.NonIndexed().Pack(
                     sender,
                     "recipient",
                     coin.Amount.BigInt(),
                 )

-                input, _ := evmhandlers.SendToIbcEvent.Inputs.NonIndexed().Pack(
+                input, _ := evmhandlers.SendCroToIbcEvent.Inputs.NonIndexed().Pack(
                     sender,
                     "recipient",
                     coin.Amount.BigInt(),
                 )

-                input, _ := evmhandlers.SendToIbcEvent.Inputs.NonIndexed().Pack(
+                input, _ := evmhandlers.SendCroToIbcEvent.Inputs.NonIndexed().Pack(
                     sender,
                     "recipient",
                     coin.Amount.BigInt(),
                 )

Also applies to: 392-397, 446-450

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@x/cronos/keeper/evmhandlers_test.go` around lines 368 - 373, The test is
packing event payloads with evmhandlers.SendToIbcEvent but the test is
specifically for SendCroToIbcHandler; change the payload packing to use the
SendCroToIbcEvent ABI (e.g.
evmhandlers.SendCroToIbcEvent.Inputs.NonIndexed().Pack(...)) wherever
SendToIbcEvent is used in TestSendCroToIbcHandler (and the other occurrences
noted), so the packed input uses the contract-specific SendCroToIbcEvent layout
instead of SendToIbcEvent.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

x/cronos/types/params.go (2)

143-154: ⚡ Quick win

Empty strings in the address slice create validation inconsistency.

validateIsEvmAddresses allows empty strings within the slice because it delegates to validateIsEvmAddress, which returns nil for empty strings (line 161-163). However, the zero-address check (lines 167-169) rejects the explicit zero address hex string. This creates an inconsistency: "" is allowed but converts to the zero address via common.HexToAddress(""), while "0x0000000000000000000000000000000000000000" is explicitly rejected.

For a slice parameter, the "not set" state should be []string{} (empty slice), not a slice containing empty strings like []string{"", ""}. Allowing empty strings within the slice serves no purpose and may confuse operators who might mistakenly believe adding "" to the list has some effect.

🛡️ Proposed fix to reject empty strings in the slice

Reject empty strings in validateIsEvmAddresses before delegating to the single-address validator:

 func validateIsEvmAddresses(i interface{}) error {
 	addrs, ok := i.([]string)
 	if !ok {
 		return fmt.Errorf("invalid parameter type: %T", i)
 	}
 	for _, addr := range addrs {
+		if len(addr) == 0 {
+			return fmt.Errorf("address list must not contain empty strings")
+		}
 		if err := validateIsEvmAddress(addr); err != nil {
 			return err
 		}
 	}
 	return nil
 }

Alternatively, remove the empty-string early return from validateIsEvmAddress if it's only used for slice elements.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@x/cronos/types/params.go` around lines 143 - 154, validateIsEvmAddresses
currently lets empty strings pass through because validateIsEvmAddress treats ""
as valid; update validateIsEvmAddresses to explicitly reject empty elements
before calling validateIsEvmAddress by checking each addr == "" and returning a
descriptive error (e.g., "empty address in slice"), so slices must use
[]string{} for "not set"; alternatively, remove the empty-string early-return in
validateIsEvmAddress but prefer the slice-level check in validateIsEvmAddresses
to catch this case.

---

161-163: 💤 Low value

Document the rationale for allowing empty strings in single-address validation.

The early return for empty strings in validateIsEvmAddress is appropriate for optional single-address parameters like CronosAdmin (where "" means "not set"), but when reused for slice validation in validateIsEvmAddresses, it produces the confusing behavior flagged above.

Consider adding a comment here explaining that empty strings are permitted to support optional single-address fields, or split the validators into separate single-address and slice-element variants.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@x/cronos/types/params.go` around lines 161 - 163, The empty-string early
return in validateIsEvmAddress currently permits "" for optional single-address
params (e.g., CronosAdmin) but causes confusing behavior when reused by
validateIsEvmAddresses; either add a clear comment above validateIsEvmAddress
explaining that the empty string is intentionally allowed to represent "not set"
for single-value params, and note that callers validating slices should handle
empties separately, or refactor by creating two validators:
validateIsEvmAddressAllowEmpty (for single optional fields) and
validateIsEvmAddressNonEmpty (used by validateIsEvmAddresses for slice
elements), then update validateIsEvmAddresses to call the non-empty variant.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes